Drop max_inbound_htlc_value_in_flight_percent_of_channel #878
Drop max_inbound_htlc_value_in_flight_percent_of_channel #878tnull merged 1 commit intolightningdevkit:mainfrom
max_inbound_htlc_value_in_flight_percent_of_channel #878Conversation
|
👋 I see @valentinewallace was un-assigned. |
54b2aa3 to
e09fd86
Compare
e09fd86 to
e8f3575
Compare
tankyleo
left a comment
There was a problem hiding this comment.
Thank you I am excited to land this PR soon, a few nits
| .unannounced_channel_max_inbound_htlc_value_in_flight_percentage, | ||
| 100 | ||
| ); | ||
| debug_assert!(config.accept_forwards_to_priv_channels); |
There was a problem hiding this comment.
nit: can we add a debug_assert here on announce_for_forwarding = false ? IIUC right now we inherit announce_for_forwarding = false here because of the default in rust-lightning, but would be good to confirm this here I think.
This is to make sure that the 100% cap actually gets applied to the channel opened by the LSPS2 service.
| // If we act as an LSPS2 service, set the HTLC-value-in-flight to 100% of the channel value | ||
| // to ensure we can forward the initial payment. | ||
| user_config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = | ||
| 100; |
There was a problem hiding this comment.
To confirm here I don't think setting 100% here would have helped as that maximum only applies to the inbound HTLCs on that channel. To make sure that 100% of the value can be allocated to outbound HTLCs from the LSP's perspective as the comment describes, the user needs to set their max_inbound to 100%.
So seems to me the intention of the code did not match the comment ?
There was a problem hiding this comment.
Hmm... yeah it does seem that way.
| let mut user_config = default_user_config(&self.config); | ||
| user_config.channel_handshake_config.announce_for_forwarding = announce_for_forwarding; | ||
| user_config.channel_config = (channel_config.unwrap_or_default()).clone().into(); | ||
| // We set the max inflight to 100% for private channels. |
There was a problem hiding this comment.
nit: similar here, would it be worth adding a debug_assert to guard against any changes away from this behavior ?
| accept_underpaying_htlcs: Some(true), | ||
| ..Default::default() | ||
| }), | ||
| ..Default::default() |
There was a problem hiding this comment.
nit: as elsewhere, maybe a quick debug_assert here on the 100% value ? Up to @tnull
e8f3575 to
4fda093
Compare
Per review feedback on PR lightningdevkit#878. The forwarding behavior for LSPS2 channels relies on LDK's default of 100% for the unannounced-channel inbound HTLC value-in-flight cap. Catch upstream changes to that default with debug asserts at the three sites that depend on it. Generated with assistance from Claude Code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| // If we act as an LSPS2 service, set the HTLC-value-in-flight to 100% of the channel value | ||
| // to ensure we can forward the initial payment. | ||
| user_config.channel_handshake_config.max_inbound_htlc_value_in_flight_percent_of_channel = | ||
| 100; |
There was a problem hiding this comment.
Hmm... yeah it does seem that way.
The max_inbound_htlc_value_in_flight_percent_of_channel config setting was used when acting as an LSPS2 service in order to forward the initial payment. However, upstream divided the config setting into two for announced and unannounced channels, the latter defaulting to 100%.
20bae9a to
d93bb39
Compare
The
max_inbound_htlc_value_in_flight_percent_of_channelconfig setting was used when acting as an LSPS2 service in order to forward the initial payment. However, upstream divided the config setting into two for announced and unannounced channels, the latter defaulting to 100%.Based on #874.